Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sys/net/gnrc: Remove code duplication #16502

Merged
merged 2 commits into from
Mar 26, 2024
Merged

sys/net/gnrc: Remove code duplication #16502

merged 2 commits into from
Mar 26, 2024

Conversation

francois-berder
Copy link
Contributor

Contribution description

While using simian (a tool that can detect code duplication) on RIOT-OS, I found two snippets of code that were good candidates for being factorized.
This pull request does not change the behavior of gnrc.

Testing procedure

I compiled for the SAMD21-XPRO board the following examples: gnrc_minimal, gnrc_networking and gnrc_networking_mac.

Issues/PRs references

None.

@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels May 27, 2021
@miri64 miri64 added the Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation label May 27, 2021
@miri64
Copy link
Member

miri64 commented May 27, 2021

I compiled for the SAMD21-XPRO board the following examples: gnrc_minimal, gnrc_networking and gnrc_networking_mac.

Since I don't have the time to test it myself at the moment, to all testers: Only gnrc_networking_mac uses the modules that are changed here and gnrc_lwmac is only used when explicitly selected in the Makefile.

@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 2, 2022
@miri64 miri64 removed the State: stale State: The issue / PR has no activity for >185 days label Mar 2, 2022
@stale
Copy link

stale bot commented Sep 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@miri64
Copy link
Member

miri64 commented Sep 21, 2022

@jia200x what do you think of this PR?

@jia200x
Copy link
Member

jia200x commented Sep 21, 2022

I think it makes sense!

@miri64
Copy link
Member

miri64 commented Sep 21, 2022

I think it makes sense!

I mean in the scope of DSME coming in and the MAC layers fixed here potentially going out soon ;-)

@Teufelchen1
Copy link
Contributor

Ping! :> Where do we stand here?

@miri64
Copy link
Member

miri64 commented Mar 26, 2024

The question still stands if those mac layers are still needed. We have DSME now, and lwmac and gomach are effectively unmaintained for years. I'd say: Let's rebase this one and merge, then deprecate lwmac and gomach.

@francois-berder @miri64
gnrc/lwmac: Reduce code duplication
7523bb0 
Both _rx_management_failed and _rx_management_success functions
attempt to sleep after handling the packet reception
failure/success. This commit extracts the sleep attempt in a
new _rx_management_attempt_sleep function.

Signed-off-by: Francois Berder <fberder@outlook.fr>
@francois-berder @miri64
gnrc/gomach: Reduce code duplication
9056e14 
The function gomach_vtdma_end is nearly identical to _no_vtdma_after_cp
expect the first few lines. This commit replaces the duplicating code
in gomach_vtdma_end by a call to _no_vtdma_after_cp.

Signed-off-by: Francois Berder <fberder@outlook.fr>
@miri64 miri64 force-pushed the remove-code-duplication branch from 630c48c to 9056e14 Compare March 26, 2024 14:30
@miri64 miri64 enabled auto-merge March 26, 2024 14:49
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 26, 2024
@riot-ci
Copy link

riot-ci commented Mar 26, 2024

Murdock results

✔️ PASSED

9056e14 gnrc/gomach: Reduce code duplication

Success Failures Total Runtime
10031 0 10031 09m:45s

Artifacts

@miri64 miri64 added this pull request to the merge queue Mar 26, 2024
Merged via the queue into RIOT-OS:master with commit 4059244 Mar 26, 2024
27 checks passed
@miri64 miri64 mentioned this pull request Mar 27, 2024
Merged
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 approved these changes

@cgundogan cgundogan Awaiting requested review from cgundogan

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann PeterKietzmann is a code owner

Assignees
No one assigned
Labels
Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
Release 2024.04
Development

Successfully merging this pull request may close these issues.
6 participants
@francois-berder @miri64 @jia200x @Teufelchen1 @riot-ci @MrKevinWeiss